Skip to content

SEP-1564: Paginate TaskExecutionApp list routes by default - #1126

Merged
yyyyyyyan merged 5 commits into
mainfrom
SEP-1564
Jul 15, 2026
Merged

SEP-1564: Paginate TaskExecutionApp list routes by default#1126
yyyyyyyan merged 5 commits into
mainfrom
SEP-1564

Conversation

@yyyyyyyan

@yyyyyyyan yyyyyyyan commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Default the shared TaskExecutionApp framework spine's derived list route to paginated, with a NO_PAGINATION sentinel as the single explicit opt-out — so a new task app gets bounded list responses without opting in.

  • Framework (app/sep/apps/framework/apps.py): the pagination field now defaults to make_pagination_dep() (page size 50, ceiling 200) instead of None. None is dropped from the public field type; a new NO_PAGINATION singleton (mirroring the existing UNGUARDED guard sentinel — same __slots__/__repr__ shape) is the single opt-out. The sentinel is resolved to None once at the spine boundary in build_router, so the internal route-derivation layer (api.py) is unchanged and keeps keying its plain-list vs envelope branch on that internal None.
  • List-shape flip: the archives, checksums, and snippets list routes (which omit the field) auto-flip from a plain list[model] to a PaginatedResponse[model] envelope. Regenerated OpenAPI snapshots, the aggregate API spec, and the generated TypeScript types capture the flip; the 5 already-paginated apps' specs are unchanged.
  • App-declaration cleanups: backup_pg and backup_mongo drop their now-redundant pagination= declarations. Their effective limit (page 50 / ceiling 200) is identical to the new default, so behavior is preserved.

Frontend consumer update (outside the original plan's scope)

Most app list pages read their list route through the generic useAppTasks/useAppEntityList hooks, which already unwrap both the flat-list and paginated-envelope shapes — that covers archives and checksums. snippets, however, is a bespoke app with its own useSnippets hook that fetched the list route and returned the body verbatim. Once the route paginates, that hook would hand the list page an envelope object where it expects an array, crashing the page. useSnippets now unwraps both shapes, so the cached query data stays SnippetResponse[] for every consumer (including the optimistic approve/unapprove mutations). A hook-level regression test exercises the real HTTP→JSON path for both the envelope and legacy-array shapes.

Tests

Default-on and NO_PAGINATION paths are covered for both CRUD-flavored and script-flavored apps, plus a pagination=None rejection test, a script-flavored paginated over-HTTP test, and a contract-suite branch fix (keys on the sentinel instead of None). Per-app list-shape assertions for the flipped apps were updated to unwrap the envelope.

Tested

  • Open the Archives list page — tasks render (first page).
  • Open the Checksums list page — items render (first page).
  • Open the Snippets list page — snippets render (no crash); approve then un-approve a snippet and confirm the row updates.
  • Open an already-paginated app list (e.g. PostgreSQL Backups) — still renders.
  • GET /api/apps/archives/ returns a {items, total, offset, limit} envelope; GET /api/apps/archives/?limit=1 returns a single-item page.
  • GET /api/apps/snippets/?offset=1 returns the next page (or an empty items list past the end) without error.

Known limitations

  • First-page-only list rendering (accepted, tracked separately). The generic and bespoke list pages fetch the list route without offset/limit or page controls, so once archives/checksums/snippets paginate, lists longer than 50 rows are truncated to the first page in the UI. This already affects the 5 previously-paginated apps, so the change makes the behavior consistent rather than introducing a new class of truncation; wiring a load-more/pagination control is out of scope here and tracked as a separate follow-up.

Checklist

  • New/modified functions have type hints and rST docstrings
  • New tests added for new features or bug fixes
  • Database migrations generated if models changed (make makemigrations) (N/A — no model changes)
  • User-facing changes documented (changelog fragment + field docstring)
  • Configuration changes documented with examples (N/A — no configuration changes)

…INATION opt-out

The pagination field now defaults to make_pagination_dep() (page 50, max 200)
so a derived list route is bounded by default; None is dropped from the public
field type and survives only as api.py's internal route-shape switch. A new
NO_PAGINATION sentinel (mirroring UNGUARDED) is the single explicit opt-out,
resolved to None once at the spine boundary so api.py is unchanged.

The archives, checksums, and snippets list routes flip from a plain list to a
PaginatedResponse envelope; backup_pg/backup_mongo drop their now-redundant
pagination declarations (effective limit unchanged).
The snippets derived list route now returns a PaginatedResponse envelope by
default. useSnippets has a bespoke fetch path (snippets is a CUSTOM_APP_REGISTRY
entry) that bypasses the generic unwrapTasks helper archives/checksums route
through, so it returned the raw envelope and the list page crashed on
for...of. Unwrap both the envelope and the legacy flat-array shape so the cached
query data stays SnippetResponse[] for every consumer.
Copilot AI review requested due to automatic review settings July 15, 2026 01:23
@yyyyyyyan yyyyyyyan added the qa in progress Someone is currently testing this PR - do not merge it label Jul 15, 2026
@yyyyyyyan yyyyyyyan self-assigned this Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the SEP plugin framework’s TaskExecutionApp so derived GET / list routes paginate by default (default limit 50, max 200), with a single explicit opt-out sentinel NO_PAGINATION. This bounds list responses for newly added task apps without requiring each app to opt in, and updates affected backend tests, OpenAPI snapshots, and frontend consumers (notably the bespoke snippets app) to handle the paginated envelope.

Changes:

  • Make TaskExecutionApp.pagination default to make_pagination_dep() and introduce NO_PAGINATION as the explicit opt-out, resolved to None at the spine boundary.
  • Update affected apps/tests/snapshots to reflect list response shape flipping from list[...] to PaginatedResponse[...] for archives/checksums/snippets.
  • Update the snippets frontend hook to unwrap either the legacy flat array or the new paginated { items, ... } envelope, with regression tests.

Reviewed changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
app/sep/apps/framework/apps.py Defaults derived list routes to paginated; adds NO_PAGINATION sentinel and resolves it in build_router.
app/sep/apps/backup_pg/app.py Removes redundant explicit pagination=make_pagination_dep() now covered by the framework default.
app/sep/apps/backup_mongo/app.py Removes redundant explicit pagination wiring now covered by the framework default.
changelog.d/SEP-1564.changed.md Documents the default pagination behavior and list response shape changes.
tests/app/sep/apps/framework/test_apps.py Extends framework tests for default pagination vs NO_PAGINATION for CRUD and script-flavored apps; adds pagination=None rejection coverage.
tests/app/sep/apps/framework/test_api.py Adds end-to-end HTTP tests for paginated script list routes and a helper to mount script routers.
tests/app/sep/apps/framework/test_scaffold.py Updates scaffold expectations to accept paginated list envelopes by default.
tests/app/sep/apps/framework/test_script_source.py Updates script-source derived-route HTTP tests for default envelope and NO_PAGINATION opt-out.
tests/app/sep/apps/framework/kit.py Adds a synthetic script-flavored app builder to exercise pagination behavior end-to-end in tests.
tests/app/sep/apps/framework/contract_suite.py Updates contract suite branching to use NO_PAGINATION instead of None for list-shape expectations.
tests/app/sep/apps/snippets/test_api_routes.py Updates snippets API list tests to unwrap items from the paginated response.
tests/app/sep/apps/archives/test_api.py Updates archives list test to unwrap items from the paginated response.
tests/app/sep/snapshots/openapi/archives.json Regenerates OpenAPI snapshot reflecting paginated list envelope + params.
tests/app/sep/snapshots/openapi/checksums.json Regenerates OpenAPI snapshot reflecting paginated list envelope + params.
tests/app/sep/snapshots/openapi/snippets.json Regenerates OpenAPI snapshot reflecting paginated list envelope + params (and 422 due to query validation).
frontend/packages/apps/snippets/src/hooks.ts Updates useSnippets to unwrap either legacy array or paginated envelope to keep consumer shape stable.
frontend/packages/apps/snippets/src/hooks.test.tsx Adds tests covering both envelope and legacy-array responses for useSnippets.
frontend/packages/api/specs/sep.json Updates aggregated OpenAPI spec to reflect pagination changes.
frontend/packages/api/src/generated/sep.ts Updates generated TypeScript types/operations to match the updated OpenAPI.

Comment thread app/sep/apps/framework/apps.py Outdated
@yyyyyyyan yyyyyyyan added qa passed Tests for this PR are completed and successful. and removed qa in progress Someone is currently testing this PR - do not merge it labels Jul 15, 2026
@yyyyyyyan

Copy link
Copy Markdown
Contributor Author

Automated QA — PASS

Verified the unchecked Tested items against a fresh instance on the PR head. The shared TaskExecutionApp list routes now return a paginated {items, total, offset, limit} envelope, and every list page consumes it correctly:

  • Archives list page renders the paginated envelope — the list page loads with its pagination footer driven by the envelope. This instance has no archive tasks, so it correctly shows the empty page (0-0 of 0) rather than erroring on the new response shape.

    Archives list page renders the empty paginated envelope

  • Checksums list page renders a populated first page — one real row renders and the pager reads 1-1 of 1, matching the envelope's total: 1. This exercises the non-empty unwrap path.

    Checksums list page with one row, pager 1-1 of 1

  • Snippets list page renders without errors — the bespoke snippets hook unwraps the envelope and renders the first page of the 89-row list.

    Snippets list first page renders 50 rows

  • Un-approving a snippet updates its row in place — removing approval flips the row's Approved cell to No and swaps the action to Approve, while sibling rows stay Yes / Remove. The optimistic mutation runs on the cached list without error.

    Un-approved snippet row shows No / Approve while siblings show Yes / Remove

  • Already-paginated PostgreSQL Backups list still renders — dropping its now-redundant pagination= declaration leaves the page rendering the envelope as before (empty here, total: 0).

    PostgreSQL Backups list still renders

  • GET /api/apps/archives/ returns the {items, total, offset, limit} envelope, and ?limit=1 is honored (limit: 1 in the response).

    GET /api/apps/archives/ returns the envelope

    GET /api/apps/archives/?limit=1 honors the limit

  • GET /api/apps/snippets/?offset=1 returns the next page without erroroffset: 1 is echoed and the page returns rows against total: 89.

    GET /api/apps/snippets/?offset=1 returns page two

  • GET /api/apps/checksums/ returns the same paginated envelope — confirms the checksums list route flipped alongside archives and snippets, not just the two explicitly named in the Tested list.

    GET /api/apps/checksums/ returns the envelope

Observations — pre-existing, out of scope

  • As the Known Limitations section notes, the list pages fetch only the first page (50 rows) with no page controls, so lists longer than 50 rows are truncated in the UI. One sharper edge worth a note for the follow-up that wires the load-more control: the snippets list's Approval filter and search run client-side over only that first page, so with 89 snippets a filter/search can miss rows past the first page (e.g. the "Not approved" filter returned nothing while un-approved snippets existed off-page). Moving those server-side alongside the page control would close the gap.

@github-actions

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  app/sep/apps/backup_mongo
  app.py
  app/sep/apps/backup_pg
  app.py
  app/sep/apps/framework
  apps.py 130
  app/sep/apps/mysql_backups/restore
  deps.py
  app/sep/sync/syncers
  pmm.py
  app/sep/sync/syncers/mysql
  syncer.py
Project Total  

This report was generated by python-coverage-comment-action

@yyyyyyyan

Copy link
Copy Markdown
Contributor Author

Tracked the follow-up from the PR description as SEP-1567: Add load-more/pagination controls to app list pages.

@yyyyyyyan
yyyyyyyan merged commit e121d71 into main Jul 15, 2026
29 of 38 checks passed
@yyyyyyyan
yyyyyyyan deleted the SEP-1564 branch July 15, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend python qa passed Tests for this PR are completed and successful.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants